Skip to content

Conversation

@MayaKirova
Copy link
Contributor

Closes #16464

Merge configs so that if user has set empty config for something, it will not override the default for that config with undefined.

Additional information (check all that apply):

  • Bug fix
  • New functionality
  • Documentation
  • Demos
  • CI/CD

Checklist:

  • All relevant tags have been applied to this PR
  • This PR includes unit tests covering all the new code (test guidelines)
  • This PR includes API docs for newly added methods/properties (api docs guidelines)
  • This PR includes feature/README.MD updates for the feature docs
  • This PR includes general feature table updates in the root README.MD
  • This PR includes CHANGELOG.MD updates for newly added functionality
  • This PR contains breaking changes
  • This PR includes ng update migrations for the breaking changes (migrations guidelines)
  • This PR includes behavioral changes and the feature specification has been updated with them

@tishko0 tishko0 self-assigned this Nov 27, 2025
@tishko0 tishko0 added ✅ status: verified Applies to PRs that have passed manual verification and removed ❌ status: awaiting-test PRs awaiting manual verification labels Dec 4, 2025
@dkamburov
Copy link
Contributor

@MayaKirova please resolve conflicts and do a pr for 21.0.x

@dkamburov dkamburov requested a review from Copilot January 9, 2026 14:17
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a bug where user-provided pin configuration would override default pinning configuration values with undefined. The fix ensures that only explicitly set properties in the user configuration replace the defaults, while unset properties retain their default values.

Changes:

  • Updated the pinning setter to merge user configuration with existing defaults using Object.assign()

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

this.resetCaches();
}
this._pinning = value;
this._pinning = Object.assign({}, this._pinning, value);
Copy link

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The updated pinning configuration merge logic lacks test coverage. Consider adding tests to verify that: 1) partial user configurations correctly merge with defaults, 2) undefined values in user config don't override existing defaults, and 3) explicitly set user values properly replace defaults.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

20.2.x grid: column-pinning ✅ status: verified Applies to PRs that have passed manual verification

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Initially pinned column is not shown in the grid when pinning config has no explicit column setting.

4 participants